Append extra_hosts from all tasks
#11074
Conversation
There was a problem hiding this comment.
I didn't notice it until now, but I'm afraid #10823 introduced a textbook time-of-check to time-of-use (TOCTOU) bug.
The key piece of prerequisite knowledge is that TaskRunners, and therefore Drivers, run concurrently within a single allocation.
Prior to #10823 hosts files had a single writer: their TaskRunner+Driver.
#10823 changed the file location so that it was shared between all tasks in an allocation: thus introducing a problem where the single file has multiple writers. Two tasks may do the Stat(...) call and both receive DoesNotExist errors, and then proceed to race to do the write.
Since the hosts file is usually quite small, the reads and writes probably all end being atomic, so the resulting file is probably always valid (no half-of-one write, half-of-another). However I think it's possible for tasks to still clobber each other's hosts files.
Sadly I'm not sure there's a simple fix. I think extra_hosts should be *on the group network stanza so it can be written once by the AllocRunner and included in a MountConfig passed to each TaskRunner+Driver. That's a pretty significant change though.
In the mean time we could document on extra_hosts is not safe to use for task groups with >1 non-lifecycle task.
|
Ahh that's a great point @schmichael. It sounds like a more comprehensive refactoring of I think I will close this for now then until we have a proper fix. |
|
There really needs to be a |
|
Closed with |
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
#10823 changed how
/etc/hoststo be mounted from the alloc dir instead of the task dir so that all tasks share the same configuration.The problem is that allocations with multiple tasks would only use the values of
extra_hostsfrom the task that starts first, ignoring all others. This prevents Consul Connect tasks from usingextra_hostsbecause the sidecar proxy doesn't define anyextra_hosts, so the main task configuration is ignored.This fix appends any
extra_hostsvalues if the hosts file already exists in the alloc dir.Closes #11056